#247 Implement basics of FlowReader #306

benedeki · 2024-12-11T12:55:16Z

Flow reader methods to read checkpoints
Page class to nicely handle paginated results
GroupedPage class to handle paginated results that can be grouped
ApiPaths moved from_Server_ to Model
Checkpoint and Measurement classes in Model to detach user from (versioned) DTOs

Closes #247

* created new module Info * the new modul added to JaCoco and CI routines

* JaCoCo exclusion for model

* created Provider to query the data from server * support for Future, IO, and ZIO based providers * work in progress

* fixed license headers

…endpoints-from-info-module

* start of work

…-implement-basics-of-flowreader # Conflicts: # .github/workflows/jacoco_report.yml # README.md # reader/src/main/scala/za/co/absa/atum/reader/FlowReader.scala # reader/src/main/scala/za/co/absa/atum/reader/PartitioningReader.scala # reader/src/main/scala/za/co/absa/atum/reader/basic/Reader.scala # reader/src/test/scala/za/co/absa/atum/reader/FlowReaderUnitTests.scala # reader/src/test/scala/za/co/absa/atum/reader/PartitioningReaderUnitTests.scala # reader/src/test/scala/za/co/absa/atum/reader/basic/PartitioningIdProviderUnitTests.scala

* `Page` class to nicely handle paginated results * `GroupedPage` class to handle paginated results that can be grouped

benedeki · 2024-12-11T13:00:19Z

model/src/main/scala/za/co/absa/atum/model/types/Checkpoint.scala

+import java.time.ZonedDateTime
+import java.util.UUID
+
+case class Checkpoint (


Wonder if this should be in Model or only in Reader?

I'm in 30% of the review only so far, but I thing that this is an internal checkpoint representation used in Reader, thus it shouldn't be in model; Reader seems like a very good place for it. The same logic applies to internal data structures used in Agent; if we put all of these into model then it would be (even more) messy (meaning, we have already quite a lot of DTOs in model :D)

benedeki · 2024-12-11T13:00:51Z

model/src/main/scala/za/co/absa/atum/model/types/Measurement.scala

+import za.co.absa.atum.model.ResultValueType
+import za.co.absa.atum.model.dto.MeasurementDTO
+
+trait Measurement {


Same question as above - should this be in Model or only in Reader?

probably same answer - if this is only gonna be used in Reader, then it should be in Reader. If it's common enough to be also used in Agent, then I'd say let's keep it in model/.

* added + function to `Page` and `GroupedPage` classes

github-actions · 2024-12-11T17:54:00Z

JaCoCo model module code coverage report - scala 2.13.11

Overall Project	43.72%	❌
Files changed	0%	❌

File	Coverage
ApiPaths.scala	0%	🍏
CheckpointWithPartitioningDTO.scala	0% `-12.12%`	❌
CheckpointV2DTO.scala	0%	❌
AtumPartitionsCheckpoint.scala	0%	❌
Measurement.scala	0%	❌
Checkpoint.scala	0%	❌

github-actions · 2024-12-11T17:54:01Z

JaCoCo agent module code coverage report - scala 2.13.11

Overall Project	78.2%	🍏

There is no coverage information present for the Files changed

github-actions · 2024-12-11T17:54:03Z

JaCoCo reader module code coverage report - scala 2.13.11

Overall Project	22.62%	❌
Files changed	5.7%	❌

File	Coverage
PartitioningIdProvider.scala	100%	🍏
Reader.scala	100%	🍏
ReaderException.scala	100%	🍏
RequestException.scala	83.02%	❌
FlowReader.scala	9.88%	❌
RequestResult.scala	4.55% `-63.64%`	❌
AbstractPage.scala	0%	❌
GroupedPage.scala	0%	❌
Page.scala	0%	❌
PaginatedResponseImplicits.scala	0%	❌
EitherImplicits.scala	0%	❌

github-actions · 2024-12-11T17:54:04Z

JaCoCo server module code coverage report - scala 2.13.11

Overall Project	68.39%	🍏

File	Coverage
CheckpointControllerImpl.scala	100%	🍏
PartitioningControllerImpl.scala	91.67%	🍏
BaseController.scala	85.29%	🍏

lsulak · 2024-12-12T11:07:45Z

model/src/main/scala/za/co/absa/atum/model/ApiPaths.scala

@@ -14,7 +14,7 @@
 * limitations under the License.
 */

-package za.co.absa.atum.server.api.http
+package za.co.absa.atum.model


yeah something inside me says that this is not really really a 'model' but in general I'm okay with this; finding a better name for 'model' is difficult and it's backward incompatible change; alternatively, creating a new model for this seems a bit too much, so I prefer your solution

lsulak · 2024-12-12T11:17:26Z

model/src/main/scala/za/co/absa/atum/model/dto/traits/CheckpointCore.scala

+ * limitations under the License.
+ */
+
+package za.co.absa.atum.model.dto.traits


hmm I'm not sure I like the location of this file; the trait/ dir has only this 1 file which doesn't really do much to our service in terms of better 'modularity' / 'structure of the DTO module. Besides, the fact that it's trait can be seen pretty nicely in IntelliJ, as it's visually different (green trait icon)

In fact, I don't really see a big value in doing this at all to be fair. I mean, what if CheckpointV3 will be different? Then we can't do this or will update the trait..I mean it seems potentially limiting without too much benefits.

Also you did it only for checkpoints, not Partitioning DTOs; to me it really seem unncessary hierarchy

The trait is a solution for inability/discouragement of case class inheritance. Because CheckpointWithPartitioningDTO is by definition an enhanced CheckpointDTO. And there's a a point, where I want to be able to handle both with the same function.

Package traits is debatable of course. Naming... 😉

lsulak · 2024-12-12T11:24:46Z

model/src/main/scala/za/co/absa/atum/model/types/Measurement.scala

+import za.co.absa.atum.model.dto.MeasurementDTO
+
+trait Measurement {
+  type T


Suggested change

type T

type V

just a suggestion

lsulak · 2024-12-12T11:29:17Z

model/src/main/scala/za/co/absa/atum/model/types/Measurement.scala

+
+object Measurement {
+
+  def apply[T](from: MeasurementDTO): Measurement = {


this has some similarity with agent's model/MeasureResult - did you check if it can be reused for Reader usages? Maybe slightly changed/moved if needed. But I think if model and agent are doing conceptually similar thing maybe we can unite and reuse some of the existing code

The similarity did occur to me. But they are not 100% same unlike AtumPartitions and AdditionalData and did want to further delay in search for commonality. I think that can be done in future, if found useful.

lsulak · 2024-12-12T14:42:20Z

reader/src/main/scala/za/co/absa/atum/reader/FlowReader.scala

+import za.co.absa.atum.model.types.{AtumPartitionsCheckpoint, Checkpoint}
+import za.co.absa.atum.reader.implicits.PaginatedResponseImplicits.PaginatedResponseMonadEnhancements
+import za.co.absa.atum.reader.implicits.EitherImplicits.EitherMonadEnhancements
+import za.co.absa.atum.reader.implicits.PaginatedResponseImplicits


this one is not needed due to line 29

lsulak · 2024-12-12T14:45:41Z

reader/src/main/scala/za/co/absa/atum/reader/FlowReader.scala

+    getQuery(endpoint, params)
+  }
+
+  private def geetCheckpointDTOs(checkpointName: Option[String], pageSize: Int = 10, offset: Long = 0): F[RequestResult[Page[CheckpointWithPartitioningDTO, F]]] = {


Suggested change

private def geetCheckpointDTOs(checkpointName: Option[String], pageSize: Int = 10, offset: Long = 0): F[RequestResult[Page[CheckpointWithPartitioningDTO, F]]] = {

private def getCheckpointDTOs(checkpointName: Option[String], pageSize: Int = 10, offset: Long = 0): F[RequestResult[Page[CheckpointWithPartitioningDTO, F]]] = {

lsulak · 2024-12-12T14:47:28Z

reader/src/main/scala/za/co/absa/atum/reader/FlowReader.scala

+      val checkpoint = Checkpoint(data)
+      AtumPartitionsCheckpoint(atumPartitions, checkpoint)
+    }
+    geetCheckpointDTOs(None, pageSize, offset).map(_.pageMap((checkpointMapper)))


Suggested change

geetCheckpointDTOs(None, pageSize, offset).map(_.pageMap((checkpointMapper)))

geetCheckpointDTOs(None, pageSize, offset).map(_.pageMap(checkpointMapper))

lsulak · 2024-12-12T14:56:32Z

reader/src/main/scala/za/co/absa/atum/reader/basic/RequestResult.scala

 import za.co.absa.atum.model.envelopes.ErrorResponse
+import za.co.absa.atum.reader.exceptions.RequestException.{CirceError, HttpException, ParsingException}
+import za.co.absa.atum.reader.exceptions.{ReaderException, RequestException}


ReaderException unused

lsulak · 2024-12-12T14:56:41Z

reader/src/main/scala/za/co/absa/atum/reader/basic/RequestResult.scala

 import za.co.absa.atum.model.envelopes.ErrorResponse
+import za.co.absa.atum.reader.exceptions.RequestException.{CirceError, HttpException, ParsingException}
+import za.co.absa.atum.reader.exceptions.{ReaderException, RequestException}
+import za.co.absa.atum.reader.result.{GroupedPage, Page}


GroupedPage unused

lsulak · 2024-12-12T15:12:48Z

model/src/main/scala/za/co/absa/atum/model/types/AtumPartitionsCheckpoint.scala

+
+import za.co.absa.atum.model.types.basic.AtumPartitions
+
+case class AtumPartitionsCheckpoint(


also possibly reader-specific?

lsulak · 2024-12-12T15:21:37Z

reader/src/main/scala/za/co/absa/atum/reader/basic/PartitioningIdProvider.scala

@@ -18,6 +18,7 @@ package za.co.absa.atum.reader.basic

 import sttp.monad.MonadError


Consider renaming package from basic to core ? just an idea

benedeki and others added 30 commits October 31, 2024 01:23

#244: Create the Info module

b97b603

* created new module Info * the new modul added to JaCoco and CI routines

* fixed License headers

e623974

* renamed to _Reader_

5e4eadb

* README.md update

2e1e2ea

* fix

738c904

* JaCoCO action update

df8c9bd

* added dummy code for testing coverage

5affd82

* erroneous class renamed

0f1e121

* JaCoCo exclusion for model

* Deleted wrong files

d773a93

#245 Add the ability to query REST endpoints from Reader module

0776f9c

* created Provider to query the data from server * support for Future, IO, and ZIO based providers * work in progress

* Work still in progress

38fde1c

* the first working commit

1ac2233

* Removed temporary notes

e6dcb52

* introduced MonadError into the GenericServerConnection

6968b02

* fixed license headers

* Fixed UTs

b9bacef

* trying to get rid of Java 11 dependency

bbb1e7f

* Downgraded sttpClient

33e6628

* further downgrade

f7ced56

* Removed exceptions

ca2116b

* commented out parts of README.md which are not yet part of the code

e5e6f63

- major rework

fe07272

* doc fix

7656f6f

Merge branch 'master' into feature/245-add-the-ability-to-query-rest-…

eb9a678

…endpoints-from-info-module

* disabled failing test

7641c07

* adjustments

bc82a5b

- further cleaning

0e7675e

* tests progress

432716a

* several UTs added

11b0a16

Merge branch 'master' into feature/245-add-the-ability-to-query-rest-…

2c3f145

…endpoints-from-info-module

* last improvements before PR ready

e07dffb

benedeki added 9 commits November 25, 2024 10:57

* description to class ServerConfig

3955a50

* removed empty line

b287a66

* addressed PR comments

d04d23b

* just better implementation

c344249

#247: Implement basics of FlowReader

c0b0988

* start of work

* major progress

55d60e1

* Further progress

5dfe5c5

* Flow reader methods to read checkpoints

67ffe07

* `Page` class to nicely handle paginated results * `GroupedPage` class to handle paginated results that can be grouped

benedeki added the work in progress Work on this item is not yet finished (mainly intended for PRs) label Dec 11, 2024

benedeki self-assigned this Dec 11, 2024

benedeki commented Dec 11, 2024

View reviewed changes

* small fixes

09e2ed8

* added + function to `Page` and `GroupedPage` classes

* License year

e7ff732

lsulak reviewed Dec 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#247 Implement basics of FlowReader #306

#247 Implement basics of FlowReader #306

benedeki commented Dec 11, 2024 •

edited

Loading

benedeki Dec 11, 2024

lsulak Dec 12, 2024

benedeki Dec 11, 2024

lsulak Dec 12, 2024

github-actions bot commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 11, 2024

github-actions bot commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading

lsulak Dec 12, 2024

lsulak Dec 12, 2024 •

edited

Loading

lsulak Dec 12, 2024 •

edited

Loading

benedeki Dec 13, 2024

benedeki Dec 13, 2024

lsulak Dec 12, 2024

lsulak Dec 12, 2024

benedeki Dec 13, 2024

lsulak Dec 12, 2024

lsulak Dec 12, 2024

lsulak Dec 12, 2024

lsulak Dec 12, 2024

lsulak Dec 12, 2024

lsulak Dec 12, 2024

lsulak Dec 12, 2024


		object Measurement {

		def apply[T](from: MeasurementDTO): Measurement = {

	private def geetCheckpointDTOs(checkpointName: Option[String], pageSize: Int = 10, offset: Long = 0): F[RequestResult[Page[CheckpointWithPartitioningDTO, F]]] = {
	private def getCheckpointDTOs(checkpointName: Option[String], pageSize: Int = 10, offset: Long = 0): F[RequestResult[Page[CheckpointWithPartitioningDTO, F]]] = {

	geetCheckpointDTOs(None, pageSize, offset).map(_.pageMap((checkpointMapper)))
	geetCheckpointDTOs(None, pageSize, offset).map(_.pageMap(checkpointMapper))


		import za.co.absa.atum.model.types.basic.AtumPartitions

		case class AtumPartitionsCheckpoint(

		@@ -18,6 +18,7 @@ package za.co.absa.atum.reader.basic

		import sttp.monad.MonadError

#247 Implement basics of FlowReader #306

Are you sure you want to change the base?

#247 Implement basics of FlowReader #306

Conversation

benedeki commented Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 11, 2024 • edited Loading

JaCoCo model module code coverage report - scala 2.13.11

github-actions bot commented Dec 11, 2024

JaCoCo agent module code coverage report - scala 2.13.11

github-actions bot commented Dec 11, 2024 • edited Loading

JaCoCo reader module code coverage report - scala 2.13.11

github-actions bot commented Dec 11, 2024 • edited Loading

JaCoCo server module code coverage report - scala 2.13.11

Choose a reason for hiding this comment

lsulak Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

lsulak Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benedeki commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading

lsulak Dec 12, 2024 •

edited

Loading

lsulak Dec 12, 2024 •

edited

Loading